[sandbox audit] Validate a pool host before sending it a credential - #4834
[sandbox audit] Validate a pool host before sending it a credential#4834Wauplin wants to merge 1 commit into
Conversation
Pool hosts were found by filtering Jobs on labels, and adopted on that basis alone. Labels are set by whoever creates the Job, and the nonce the host token is derived from is itself a public label -- so any namespace member who can create a Job could publish one carrying another user's pool labels and that host's nonce, and the victim's client would adopt it and send it the token derived for the *real* host. The Jobs proxy strips `Authorization` but forwards `X-Sandbox-Token`, so the impostor receives exactly the capability the real host accepts. Nothing in the old path bound a Job to its creator, image, flavor, command or URL. `_discover_hosts` even claimed in its docstring to match on image/flavor; it did not. Adoption now requires everything the *backend* asserts about a Job to line up, not just the labels its creator chose: - `initiator` -- the load-bearing check, and the one field `run_job` gives no way to set, so it cannot be forged client-side; - owner namespace, image (normalized for registry prefixes and case), flavor, and that the command is the sandbox bootstrap; - exactly one exposed URL, over HTTPS, whose hostname is derived from *this* job's id -- so a Job cannot name where the credentials should go. New `adopt_hosts` argument on `SandboxPool` and `SandboxPool.connect`: `"own"` (default) adopts only this principal's hosts, `"namespace"` restores cross-user sharing for namespaces whose members trust each other, `"never"` disables adoption. A refusal names the opt-in, so someone legitimately sharing hosts is not left guessing. `SandboxPool.connect`'s cold path gets the same treatment: it rebuilds the pool's entire configuration from a discovered host's spec, so an unvalidated match would let a Job in the namespace choose the image the pool's next hosts boot. `_connect_host` addresses a job the caller named explicitly, so ownership there is the caller's assertion, but the URL and bootstrap checks still apply. Separately, the HF bearer is no longer frozen when the transport is built. A host lives up to 24h, and an OIDC/OAuth credential can expire inside that window, after which every request failed even though the handle was fine. An `httpx.Auth` now reads it per request. The `X-Sandbox-Token` is deliberately *not* re-derived: the server holds the value derived from the bearer that created the job, so re-deriving from a rotated bearer would produce a token it has never seen. The two credentials answer different gates and only the bearer needs refreshing. (A host created under one HF token therefore stays reachable only by holders of that token -- inherent to deriving the credential, and now stated in the docs.) The discovery tests were part of the problem: their Job fixtures set only labels, so they asserted that a label match is sufficient. They now describe a Job the way the Jobs API really does, and a new suite covers the hostile cases -- an impostor's initiator, a mismatched image/flavor/command/namespace, a URL belonging to another job, a plain-HTTP URL, extra exposed ports, and a missing initiator (refused rather than assumed). Validation: 55 tests pass, up from 44. Writing them caught two real bugs in this change -- `urlparse` lowercases hostnames, which broke the URL check on a mixed-case job id, and a fixture whose host had no capacity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 04c3deb. Configure here.
| "Authorization": f"Bearer {self._auth_token}", | ||
| "X-Sandbox-Token": sandbox_token, | ||
| }, | ||
| headers={"X-Sandbox-Token": sandbox_token}, |
There was a problem hiding this comment.
Dead _auth_token assignment after removing all readers
Low Severity
self._auth_token is assigned via _effective_token(api) but is never read anywhere in the codebase. This PR removed both consumers — the httpx.Client Authorization header and the proxy_headers property — leaving this as a dead store that also triggers an unnecessary token resolution on every _SandboxServer construction.
Reviewed by Cursor Bugbot for commit 04c3deb. Configure here.
| # A sandbox host runs the bootstrap script and nothing else. A Job running | ||
| # anything else is not a host, whatever its labels say. | ||
| if job.command is not None and list(job.command) != _bootstrap_command(): | ||
| return "does not run the sandbox bootstrap command" |
There was a problem hiding this comment.
Null command/flavor silently bypass host validation checks
Low Severity
The flavor check (if flavor is not None and job.flavor is not None) and command check (if job.command is not None) both silently pass when the job field is None. The PR table states a job "running something else is not a host," but a job with no command is equally not verifiably running the bootstrap — yet it passes. Under ADOPT_NAMESPACE policy (where initiator is not checked), this is an inconsistency with the stated security model.
Reviewed by Cursor Bugbot for commit 04c3deb. Configure here.
Wauplin
changed the title
1 participant
Why
Pool hosts were found by filtering Jobs on labels — and adopted on that basis alone. Labels
are set by whoever creates the Job, and the nonce the host token is derived from is itself a
public label. So any namespace member who can create a Job could publish one carrying another
user's pool labels plus that host's nonce, and the victim's client would adopt it and send it
the token derived for the real host. The Jobs proxy strips
Authorizationbut forwardsX-Sandbox-Token, so the impostor receives exactly the capability the real host accepts.Nothing in the old path bound a Job to its creator, image, flavor, command or URL.
_discover_hostseven claimed in its docstring to match on image/flavor — it didn't.Approach
Adoption now requires everything the backend asserts about a Job to line up, not just the
labels its creator chose:
initiatorrun_jobhas noinitiatorparameter — the backend derives it from the authenticated caller. This is the load-bearing check.New
adopt_hostsargument onSandboxPoolandSandboxPool.connect:"own"(default) — only hosts this principal started. Safe in a shared namespace."namespace"— restores cross-user host sharing, for namespaces whose members trust eachother. Still enforces the spec checks.
"never"— no adoption; only hosts this handle booted.A refusal names the opt-in, so someone legitimately sharing hosts isn't left guessing:
SandboxPool.connect's cold path gets the same treatment — it rebuilds the pool's entireconfiguration from a discovered host's spec, so an unvalidated match would let a Job in the
namespace choose the image the pool's next hosts boot.
_connect_hostaddresses a job thecaller named explicitly, so ownership there is the caller's assertion, but the URL and
bootstrap checks still apply.
Also: the bearer is no longer frozen (the L-01 half)
The HF bearer was captured once when the transport was built. A host lives up to 24h, and an
OIDC/OAuth credential can expire inside that window, after which every request failed even
though the handle was fine. An
httpx.Authnow reads it per request.The
X-Sandbox-Tokenis deliberately not re-derived. The server holds the value derivedfrom the bearer that created the job, so re-deriving from a rotated bearer would produce a
token it has never seen. The two credentials answer different gates — the proxy wants a live
bearer, the server wants the original capability — and only the first needs refreshing.
The consequence worth stating plainly: a host created under one HF token stays reachable only
by holders of that token. That is inherent to deriving the credential from the bearer, not
something this PR can fix, and it is now in the docs rather than being a surprise.
The tests were part of the problem
The discovery fixtures set only labels — so they asserted that a label match is sufficient,
which is the bug. They now describe a Job the way the Jobs API really does, and a new
TestHostAdmissionsuite covers the hostile cases: an impostor's initiator, a mismatchedimage / flavor / command / namespace, a URL belonging to another job, a plain-HTTP URL, extra
exposed ports, and a missing initiator (refused rather than assumed). Plus an end-to-end
create()where the impostor is listed, matches on labels, and is not adopted.Validation
urlparselowercases hostnames (breakingthe URL check on a mixed-case job id), and a fixture whose host had zero capacity.
Not done here
The durable fix is a backend-signed attestation binding a Job to its creator, image
digest, pool and exposed port. This is the client asserting things about a Job it fetched —
good defence in depth, and it closes the practical attack, but it is still the client's
judgement. Worth filing with the Jobs team;
initiatorbeing non-settable is what makes theinterim version meaningful, and an authoritative
job.creatorplus server-side immutabilityfor a reserved
hf-sandbox-*label prefix would close it properly.Behaviour changes
adopt_hosts="namespace". Workflows that relied onpicking up a colleague's warm host will boot their own host (and bill for it) until they opt
in. This is the intended trade; the error message points at the flag.
proxy_headersresolves the HF token on access rather than returning a captured one.Note
High Risk
Changes authentication and credential delivery for sandbox pools (host discovery and HTTP auth), which are security-sensitive; default behavior also shifts so colleagues’ hosts are no longer adopted without
adopt_hosts="namespace".Overview
Closes a pool host impersonation hole where any namespace member could create a Job with another pool’s labels (and copied nonce) and receive the victim’s host
X-Sandbox-Token. Adoption no longer trusts labels alone:_host_rejectionrequires backend-asserted initiator (defaultadopt_hosts="own"), matching image/flavor/bootstrap command, and a single HTTPS expose URL tied to that job id.SandboxPool/connectgainadopt_hosts("own"|"namespace"|"never"); cross-user warm-host sharing needs an explicit"namespace"opt-in, with errors that mention it.Long-lived handles no longer freeze the Jobs proxy bearer:
_SandboxAuthresolves the HF token on each request;proxy_headersdoes the same on read.X-Sandbox-Tokenis unchanged (still derived from the token that created the job).Docs update the threat model and token/
proxy_headersbehavior; tests addTestHostAdmissionand realistic host job fixtures.Reviewed by Cursor Bugbot for commit 04c3deb. Bugbot is set up for automated code reviews on this repo. Configure here.